feat(cursor-review): reuse check-pr-size classifier + discount comments in the size gate#59
feat(cursor-review): reuse check-pr-size classifier + discount comments in the size gate#59mattmillerai wants to merge 3 commits into
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
…n the size gate and the reviewed diff cursor-review kept its own hand-maintained diff_excludes pathspec list to decide what codegen to skip — weaker than, and drifting from, the PR-size cap's classifier. It matched filename globs only, so generated files caught by the cap's base-ref linguist-generated attribute or the canonical Go `// Code generated ... DO NOT EDIT.` marker still counted toward the review size cap and got fed to the panel, while blunt directory excludes hid hand-written files that should have been reviewed. Make the shared scripts/check-pr-size tool the single source of truth for "what is codegen" in cursor-review too: - check-pr-size gains an opt-in --generated-out flag that writes the classified generated paths (NUL-separated). The size verdict is unchanged; pr-size.yml is unaffected (additive, default off). - diff-size builds and runs the tool once (GOWORK=off, mirroring pr-size.yml): its over_cap output (counted vs --max = DIFF_SIZE_CAP) drives within_cap, and the generated-paths list drives a single reviewed-diff artifact that excludes every classified generated file. - the panel cells and the judge consume that one artifact instead of each regenerating the diff, so they need no PR checkout — the only PR-authored content they see is the diff text they review. - new extra_generated_globs / extra_lockfiles inputs mirror pr-size and feed the classifier (count + diff). diff_excludes stays as a diff-only, back-compat escape hatch; its old lockfile/vendored default moves into the classifier and extra_generated_globs. Fails open: if the tool errors, over_cap is unset (within_cap=true) and the diff falls back to diff_excludes-only — never a silent skip. Callers that listed generated paths in diff_excludes should move them to extra_generated_globs to keep them out of the size gate too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e9cda35 to
4077d73
Compare
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 8 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 5 |
| 🟢 Low | 2 |
Panel: 8/8 reviewers contributed findings.
… (opt-in) AI-generated code now leaves a non-trivial volume of comments, which inflate the changed-line count and can push a PR over the review cap — skipping the review of what may be modest actual logic. Add an opt-in `--ignore-comments` flag (env PR_SIZE_IGNORE_COMMENTS) that excludes blank and comment-only changed lines from the counted total: - annotateDiscounts fetches the patch for the counted (non-generated, non-binary) paths only, and ParseDiscounts tallies each file's blank/comment-only added and removed lines. FileChange.Discounted carries the per-file total; Counted() = Changed() - Discounted (clamped ≥ 0). Default 0, so behavior is unchanged when the flag is off — Counted() == Changed() for every existing path and test. - Heuristic and count-only by design: a per-language, single-line comment-marker table (C-family //, hash #, SQL/Lua --, HTML <!-- -->) plus single-line block comments and blank lines. Multi-line block-comment bodies and string literals are NOT lexed (documented limitation). A line whose content merely CONTAINS a comment token (e.g. a string literal) still counts. Generated files are never inspected, and the reviewed diff is untouched — only the count changes. - Fails safe: any git/parse error leaves Discounted at 0 (count raw), so a bug can only make the cap stricter, never sneak a large change under it. cursor-review enables it via a new `ignore_comments` input (default true): AI comment volume no longer trips the diff-size gate and skips the review, but the panel still sees the comments — so it can still flag wrong or misleading ones. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address all 8 cursor-review panel threads on #59: - Go generated marker is now honored only from the BASE blob for the review gate (new --marker-from-base flag): a PR can no longer hide hand-written code from the panel by prepending the one-line marker, matching the base-ref invariant of the linguist-generated path. pr-size.yml (flag off) is unchanged. - The reviewed diff is built by the tool itself (--reviewed-diff-out): one full git diff streamed through a section filter keyed on the classified set (FilterPatch). No per-file argv pathspecs, so an unbounded generated set can't overflow ARG_MAX; a generated rename drops as one section (no orphaned old-path deletion); and a write failure exits 2 so counts are never paired with a missing/partial diff. (git diff has no --pathspec-from-file, hence post-filtering.) - Degraded-path semantics made explicit and accurate: git-level breakage fails the diff-size job (fail closed); tool-level failure falls back to a RAW no-exclusions count gate before building the diff_excludes-only diff, so a degraded run can never feed an unbounded diff to the eight panel jobs. - classify() applies path/attr rules to binary files (content read still skipped), so a binary matching extra_generated_globs (e.g. data/object_info.json.gz) leaves the reviewed diff again. - globRegexp compiles **/ to (?:.*/)? (gitignore semantics): root-level node_modules/, dist/, *.min.js now match the default excludes. - Pin every remaining mutable action tag in cursor-review.yml (checkout v6.0.3, upload-artifact v7.0.1, download-artifact v8.0.1, create-github-app-token v3.2.0). Validation: go test ./... (new FilterPatch/parseDiffGitPaths/ marker-from-base/binary-extras/glob suites), gofmt, go vet, actionlint (incl. shellcheck), python unittest (40 cursor-review tests), plus an end-to-end smoke: head-added-marker attack file is counted AND reviewed; base-generated mock excluded; root node_modules excluded; diff_excludes stays diff-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
Two related changes so cursor-review's size gate counts only the lines a human/AI reviewer actually needs to read: (1) reuse the PR-size classifier so generated code stops counting and stops being reviewed, and (2) discount blank/comment lines from the count so AI comment volume doesn't trip the cap.
ELI5
When a PR is huge, cursor-review skips it (too big for one useful pass), and whatever it does review, it feeds the models the whole diff. Two things inflate "huge": generated code (nobody hand-writes or reviews it) and, increasingly, AI-written comments (verbose, low-risk). Both were counting against the size limit and — for codegen — getting shoved at the review models.
This PR teaches the size gate to ignore both: codegen via the same detector the PR-size cap already uses, and comments via a per-language heuristic. Codegen is dropped from the reviewed diff too; comments are kept in the diff (a reviewer should still catch a wrong or misleading comment) — only the count ignores them.
Part 1 — reuse the classifier for codegen (commit 1)
cursor-review kept its own hand-typed
diff_excludespathspec list, weaker than and drifting from the PR-size cap's classifier (it matched filenames only, missing base-reflinguist-generatedand the Go// Code generated … DO NOT EDIT.marker, while blunt directory excludes hid hand-written files).check-pr-sizegains an opt-in--generated-outflag (emits the classified generated paths; size verdict unchanged,pr-size.ymluntouched).diff-sizebuilds/runs the tool once:over_capdriveswithin_cap, and the generated-paths list drives a single reviewed-diff artifact with codegen excluded.extra_generated_globs/extra_lockfilesinputs mirrorpr-size.yml;diff_excludesstays as a diff-only back-compat escape hatch.Part 2 — discount blank/comment lines from the count (commit 2)
check-pr-sizegains--ignore-comments(envPR_SIZE_IGNORE_COMMENTS): blank and comment-only changed lines are subtracted from the counted total.FileChange.Discountedcarries the per-file count;Counted() = Changed() − Discounted, default 0 → behavior unchanged when off.//,#,--,<!-- -->) + single-line block comments + blanks. Multi-line block-comment bodies and string literals aren't lexed (documented); a comment token merely contained in code still counts. Generated files are never inspected; the reviewed diff is untouched.ignore_commentsinput (default true).Behavior / back-compat
pr-size.ymlis not modified (the comment-discount capability is available for it to adopt later).diff_excludesdefault (lockfiles + vendored dirs) now lives in the classifier built-ins + theextra_generated_globsdefault → equivalent-or-better exclusion, no diff-content change for default callers.diff_excludes: move generated entries intoextra_generated_globsso they leave the size gate too. Bump the pinned SHA at the same time. Detector-caught codegen (Go markers, base-reflinguist-generated) needs no change.Validation
go test ./...inscripts/check-pr-size— new suites (ParseDiscounts,IsInsignificantLine,CommentSyntaxFor,EvaluateDiscount,AnnotateDiscountsGit,WriteGeneratedPaths) + existing pass ✅gofmt -lclean,go vetclean ✅python3 -m unittest discover .github/cursor-review/tests(40 tests, unchanged) ✅actionlint .github/workflows/cursor-review.yml(incl. shellcheck) ✅Coordination — two in-flight PRs touch this file
cursor-review-xllabel that bypasses the cap for a genuinely huge PR. This PR shrinks what counts toward the cap in the first place. They compose; whoever lands second rebasesdiff-size.Draft until we pick the merge order.